Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid transpiling class-properties as they are natively supported #449

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jun 26, 2022

This PR is incomplete -- current status: babel problem

goal: make babel do less:
tested at: https://github.com/NullVoxPopuli/babel-transpilation-tests
current problem: (all in babel, not ember-cli-babel) decorator plugin assumes class-properties plugin is needed (it's not)

This is not intended to be merged, but serves as something for folks to point their package.jsons at when they want to see if they can disable class-property transpilation across their whole app and addons.

hopefully: class property transformation is not required for decorators .

(it used to be required tho).
There is an assertion / error that's added by the decorators plugin tho.

Upon manual removal in the above demo everything still works fine.

Here is the the test:

input
function f(target, key, desc) {
  let oldGet = desc.get
  let oldInit = desc.initializer

  desc.get =() => {
    console.log('called f on ', key);
    return oldInit?.() || oldGet();
  }
}
class A {
  #a = 'hi';

  get #b() {
    return 'hello' + this.#a;
  }

  get c() { return this.#b; }

  @f g;

  @f h = 2;

  i = 3;
}

let a = new A();

console.log({ g: f.g, c: a.c, h: a.h, i: a.i });
output

remove lines which define g and h

"use strict";

var _class, _descriptor, _descriptor2;

function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }

function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'proposal-class-properties is enabled and runs after the decorators transform.'); }

function f(target, key, desc) {
  let oldGet = desc.get;
  let oldInit = desc.initializer;

  desc.get = () => {
    console.log('called f on ', key);
    return oldInit?.() || oldGet();
  };
}

let A = (_class = class A {
  #a = 'hi';

  get #b() {
    return 'hello' + this.#a;
  }

  get c() {
    return this.#b;
  }

  g = _initializerWarningHelper(_descriptor, this);
  h = _initializerWarningHelper(_descriptor2, this);
  i = 3;
}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "g", [f], {
  configurable: true,
  enumerable: true,
  writable: true,
  initializer: null
}), _descriptor2 = _applyDecoratedDescriptor(_class.prototype, "h", [f], {
  configurable: true,
  enumerable: true,
  writable: true,
  initializer: function () {
    return 2;
  }
})), _class);
let a = new A();
console.log({
  g: f.g,
  c: a.c,
  h: a.h,
  i: a.i
});

the goal of this effort is the following:

  • to reduce the amount of work babel is doing
  • to use one babel / ember-cli-babel instance for the whole app tree (the complexity brought by addons compliing themselves with their own ember-cli-babel has been problematic for app-land control).

To do this, a consuming app must have only one ember-cli-babel instance in the whole node_modules graph.

Related issues:

@NullVoxPopuli NullVoxPopuli force-pushed the stop-transpiling-class-properties branch from 3d12113 to f8adaf2 Compare June 26, 2022 21:15
@NullVoxPopuli
Copy link
Contributor Author

This won't be possible. best place we can spend effort is moving to platform-decorators

@NullVoxPopuli NullVoxPopuli deleted the stop-transpiling-class-properties branch September 23, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant